[in_app_purchase] Define clang module for iOS by jmagman · Pull Request #2125 · flutter/plugins

jmagman · 2019-09-30T21:40:37Z

Description

Limit the supported podspec platform to iOS so tests don't run (and fail) for macOS.
Define the module by setting DEFINES_MODULE in the podspec. See CocoaPod modular headers docs.
Explicitly set VALID_ARCHS to architectures included in the Flutter universal binary. This is the CocoaPods-suggested workaround to prevent tests from running on the i386 simulator. See Fix linting when armv7 is included but i386 isn't CocoaPods/CocoaPods#8159.
Fix PaymentQueueTest.m. It doesn't compile and it wasn't actually included in the example Xcode project.
Fix a few analyzer warnings:
- nullability warning in FIAPReceiptManager
- NSNumber warning to check against nil or -boolValue

Related Issues

Fixes flutter/flutter#41648.
See also flutter/flutter#41007.

Tests

Add a test_spec to the podspec so the CocoaPods linter will test them. These tests fail before the s.platform and s.pod_target_xcconfig changes.
Move existing tests to new Tests directory so they are run on pre-submit!
Fix availability in tests so they pass on older simulators.

Checklist

Breaking Change

Yes, this is a breaking change (please indicate a breaking change in CHANGELOG.md and increment major revision).
No, this is not a breaking change.

jmagman · 2019-09-30T21:56:47Z

packages/in_app_purchase/ios/Tests/Stubs.h

+#import "InAppPurchasePlugin.h"
+
+NS_ASSUME_NONNULL_BEGIN
+API_AVAILABLE(ios(11.2), macos(10.13.2))


Added availability.

jmagman · 2019-09-30T21:57:19Z

packages/in_app_purchase/ios/Tests/TranslatorTest.m

+    @"localizedTitle" : @"title",
+    @"localizedDescription" : @"des",
+  }];
+  if (@available(iOS 11.2, *)) {


Added availability so these tests passed with older SDKs.

jmagman · 2019-09-30T21:58:30Z

packages/in_app_purchase/ios/Tests/InAppPurchasePluginTest.m

@@ -0,0 +1,206 @@
+// Copyright 2019 The Chromium Authors. All rights reserved.


These test files were moved from the example/ project to the new Tests/ directory. Added symlinks from the old location so the tests still pass in the example app.

Turns out Xcode really didn't link the symlinking. I changed the Xcode project to just point at the new location instead of following the symlinks.

jmagman · 2019-09-30T22:01:06Z

packages/in_app_purchase/example/ios/in_app_purchase_pluginTests/PaymentQueueTest.m

@@ -1,183 +0,0 @@
-// Copyright 2019 The Chromium Authors. All rights reserved.


@cyanglaz This test doesn't compile and wasn't included in the Xcode project, so I deleted it. I can file an issue for it to be re-added and fixed to pass, if you still find value in it.

Yes, please add this test case back to the project and remove the .testing part to fix the compilation. I am not sure why it would be like this in the first place.

mklim

LGTM, but @cyanglaz should also review and approve.

cyanglaz

LGTM overall. Left some comments.

cyanglaz · 2019-09-30T22:27:54Z

packages/in_app_purchase/ios/Classes/InAppPurchasePlugin.m

    payment.applicationUsername = [paymentMap objectForKey:@"applicationUsername"];
    NSNumber *quantity = [paymentMap objectForKey:@"quantity"];
-    payment.quantity = quantity ? quantity.integerValue : 1;
+    payment.quantity = (quantity != nil) ? quantity.integerValue : 1;


what is the different or benefit we gain with this change?

It fixes a static analyzer warning:

157:24: warning: Converting a pointer value of type 'NSNumber *' to a primitive boolean value; instead, either compare the pointer to nil or call -boolValue payment.quantity = quantity ? quantity.integerValue : 1;

cyanglaz · 2019-09-30T22:29:05Z

packages/in_app_purchase/ios/Tests/Stubs.h

+@interface FIAPReceiptManagerStub : FIAPReceiptManager
+@end
+
+API_AVAILABLE(ios(7.0), macos(10.9))


Flutter supports 8 and up so this might not be necessary. I'm ok leave it though.

I'll remove it, most of the others should also have API_AVAILABLE(ios(7.0), macos(10.9)) so this isn't consistent.

cyanglaz · 2019-09-30T22:31:46Z

packages/in_app_purchase/pubspec.yaml

 author:  Flutter Team <flutter-dev@googlegroups.com>
 homepage: https://github.com/flutter/plugins/tree/master/packages/in_app_purchase
-version: 0.2.1+4
+version: 0.2.2


If there is no public API change, we just need to update the patch, thus 0.2.1+5
I might have missed something though. Please point it out if so.

The only public API change was adding the nullability return value. The podspec change means this will be imported as a module, so modulemaps get generated, and CocoaPods does a lot of other magic. I'm happy to change this to 0.2.1+5 though!

…pecially deployment target

jmagman requested review from cyanglaz and mklim September 30, 2019 21:40

googlebot added the cla: yes label Sep 30, 2019

jmagman changed the title ~~[in_app_purchase] Define clang module for iOS~~ WIP [in_app_purchase] Define clang module for iOS Sep 30, 2019

jmagman force-pushed the iap-module branch from a15ed45 to 3c99ec8 Compare September 30, 2019 21:49

jmagman changed the title ~~WIP [in_app_purchase] Define clang module for iOS~~ [in_app_purchase] Define clang module for iOS Sep 30, 2019

jmagman commented Sep 30, 2019

View reviewed changes

mklim reviewed Sep 30, 2019

View reviewed changes

cyanglaz approved these changes Sep 30, 2019

View reviewed changes

[in_app_purchase] Define clang module for iOS

43387af

jmagman force-pushed the iap-module branch from 99b7bc4 to 43387af Compare September 30, 2019 22:51

Convert to @imports to test module

205feca

jmagman mentioned this pull request Sep 30, 2019

[instrumentation_adapter] Report Flutter test result via XCTest #2117

Closed

13 tasks

Remove redundant module test, remove test build setting overrides (es…

b78f1b6

…pecially deployment target

cyanglaz approved these changes Oct 1, 2019

View reviewed changes

jmagman merged commit 6070d19 into flutter:master Oct 1, 2019

jmagman deleted the iap-module branch October 1, 2019 01:25

mormih pushed a commit to mormih/plugins that referenced this pull request Nov 17, 2019

[in_app_purchase] Define clang module for iOS (flutter#2125)

7fe85bb

sungmin-park pushed a commit to sungmin-park/flutter-plugins that referenced this pull request Dec 17, 2019

[in_app_purchase] Define clang module for iOS (flutter#2125)

9827291

jmagman mentioned this pull request Sep 13, 2022

[pigeon] simplify apple platform tests with pod lib lint flutter/flutter#111399

Closed

		@@ -0,0 +1,206 @@
		// Copyright 2019 The Chromium Authors. All rights reserved.

		@@ -1,183 +0,0 @@
		// Copyright 2019 The Chromium Authors. All rights reserved.

Conversation

jmagman commented Sep 30, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issues

Tests

Checklist

Breaking Change

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jmagman Sep 30, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cyanglaz Sep 30, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mklim left a comment

Choose a reason for hiding this comment

Uh oh!

cyanglaz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jmagman commented Sep 30, 2019 •

edited

Loading

jmagman Sep 30, 2019 •

edited

Loading

cyanglaz Sep 30, 2019 •

edited

Loading